Skip to content

fix(dialogs): unable to show nested frameless modal views #1469

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Aug 2, 2018

Conversation

ADjenkov
Copy link
Contributor

@ADjenkov ADjenkov commented Aug 1, 2018

The problem: Unable to show modal from another modal if they are not presented from <p-r-o> (frameless). There is no available viewController (iOS) from whom we can present future modal view.

The solution: Use previous modal child view as a parentView when presenting new modal. The child view a.k.a _ngDialogRoot should have available viewController (iOS) created from the previous modal presentation.

  • modal-navigation-ng tests added

Fix: NativeScript/NativeScript#5924

@ADjenkov ADjenkov added the bug label Aug 1, 2018
@ADjenkov ADjenkov self-assigned this Aug 1, 2018
@ghost ghost added the in progress label Aug 1, 2018
@ADjenkov
Copy link
Contributor Author

ADjenkov commented Aug 1, 2018

test branch_tns_core_modules#djenkov/proxy-view-container

@ADjenkov ADjenkov requested a review from vakrilov August 1, 2018 08:21
@ADjenkov ADjenkov force-pushed the djenkov/nested-modals branch from 51bb4d5 to 4be8f16 Compare August 1, 2018 11:37
@SvetoslavTsenov
Copy link
Contributor

test

@ADjenkov ADjenkov merged commit 6155f02 into master Aug 2, 2018
@ghost ghost removed bug in progress labels Aug 2, 2018
@ADjenkov ADjenkov deleted the djenkov/nested-modals branch August 2, 2018 10:38
@RoyiNamir
Copy link

RoyiNamir commented Aug 3, 2018

@ADjenkov
When a commit is merged to master (like in here ^)

Are we able now to use it via nativescript-angulr@next ? or @rc ?

In other words - how can we test it ^ now ?

@ADjenkov
Copy link
Contributor Author

ADjenkov commented Aug 3, 2018

Hi @RoyiNamir,
Yes you can now give it a try with nativescript-angulr@next (any feedback will be much apprеciated) .

@RoyiNamir
Copy link

@ADjenkov Thanks for reply. Question now is : should we only @next nativescript-angular , or are there any more package json libs that we should also @next ?

Basically we update a version via tns create aaa --ng and compare diffs.

Will @nexting ONLY nativescript-angular is sufficient ?

@ADjenkov
Copy link
Contributor Author

ADjenkov commented Aug 3, 2018

@RoyiNamir I'm not sure what your current deps are, but you can assure correct deps by automatically updating them with this. In your case just ingore the npm i nativescript-angular@latest --save part

@RoyiNamir
Copy link

RoyiNamir commented Aug 9, 2018

@ADjenkov Hi. - Update :

Works, but it take a serious amount of time until the second modal opens.

btw - this -

    npm i nativescript-angular@next --save

Was not sufficient , so I did this also ( like you said) 👍

    node_modules\.bin\update-app-ng-deps       (windows...)
    npm i

Thank you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants